Skip to content

Conversation

@ahejlsberg
Copy link
Member

@ahejlsberg ahejlsberg commented Apr 27, 2018

This PR removes primitive types string, number, and symbol from intersections that also contain literal types from the same domain. For example string & 'a' is reduced to just 'a'. Likewise, 10 | number is reduced to just 10.

The PR eliminates the TypeIncludes enum from the type checker in favor of just using TypeFlags. This allows us to more efficiently compute the combined flags during the construction of a union or intersection type.

@ahejlsberg ahejlsberg requested review from mhegazy and weswigham April 27, 2018 23:57
Copy link
Member

@weswigham weswigham left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like a straightforward refactor of TypeIncludes -> TypeFlags plus the small change to simplify intersections. One comment about a typo on the test, though, which may impact that strictNullChecks case.

@@ -0,0 +1,15 @@
// @strict
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

// @strict: true
This isn't setting any compiler options as is and becomes just a comment in the output. Though I guess it's heartening to know that nothing in this change actually relies on strict. 😛

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think so. We have hundreds of tests that include // @strict: true and they definitely behave differently if you remove the comment.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

....this comment isn't // @strict: true, it's just // @strict. This comment does nothing.

return includes;
}

function removeRedundantPrimtiveTypes(types: Type[], includes: TypeFlags) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Typo: should be removeRedundantPrimitiveTypes

@mhegazy
Copy link
Contributor

mhegazy commented Apr 28, 2018

Fixes #9410
Fixes #23651

@ahejlsberg ahejlsberg merged commit 25d5952 into master Apr 28, 2018
@ahejlsberg ahejlsberg deleted the reduceIntersectionTypes branch April 28, 2018 18:43
@microsoft microsoft locked and limited conversation to collaborators Jul 31, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants